Skip to content

refactor(scripts): build script hardening (SL-006, 008-013) - #477

Merged
jackgranatowski merged 1 commit into
claude/pr-469-audit-rebase-ggp0e4from
claude/audit-pr3-scripts-hardening
Jul 2, 2026
Merged

refactor(scripts): build script hardening (SL-006, 008-013)#477
jackgranatowski merged 1 commit into
claude/pr-469-audit-rebase-ggp0e4from
claude/audit-pr3-scripts-hardening

Conversation

@jackgranatowski

Copy link
Copy Markdown
Contributor

PR3 from the technical-debt audit remediation plan (#469) — the plan's own "highest technical risk" item in this wave. Targets the integration branch, same stacking model as PR1/PR2.

Findings addressed

  • SL-006 (the risky one) — split gen-api-index.js's 829 lines along its existing section-comment boundaries into scripts/lib/api-index/extract.js (annotations, previous-index reading, banner/comment parsing, bundle mapping, token/class extraction and merging) and render.js (the Markdown companion). gen-api-index.js is now just path constants, the write-time regression guard, and orchestration in main(). One small behavior-neutral cleanup along the way: replaced a hoisted-closure mutable-variable hack (bundlesFor_) with a normal function parameter — same value, same call order, just less confusing to read.
  • SL-010 + SL-012 — added regression guards before the three fs.writeFileSync sites that downstream tooling trusts (gen-api-index.js, gen-token-registry.js, gen-token-index.js). A >20% entry-count drop (or, for the append-only token-registry.json, any drop at all) now throws instead of silently writing a truncated artifact.
  • SL-008/009/011/013 — new scripts/README.md documenting the (intentionally unenforced) error-handling convention, the hand-rolled CLI flag parsing, the 3 environment variables the scripts read, and the execSync(shell:true) pattern.

Verification (this is the load-bearing check for SL-006)

Diffed docs/api-index.json, docs/api-index.md, docs/token-index.json, and token-registry.json byte-for-byte against a pre-refactor snapshot — empty diff on all four, both from a standalone node scripts/gen-api-index.js run and from the full npm run docs pipeline.

  • npm run check:macros, check:registry, check:llm-guide, audit:check — all pass
  • npm run lint:css, npm run build — clean
  • npm run test:unit — 44/44 pass

A process note

While verifying this PR I hit the exact bug Qodo's review just flagged on PR #476: npm run test:unit fails if badges/ isn't freshly built, and that's not obvious from the script name. I'll fix that documentation gap over on #476.


Generated by Claude Code

…, 012, 013)

- SL-006 (highest-risk item in this wave): split gen-api-index.js's 829
  lines along its existing section-comment boundaries into
  scripts/lib/api-index/extract.js (annotations, previous-index reading,
  banner/comment parsing, bundle mapping, token/class extraction and
  merging) and render.js (the Markdown companion). gen-api-index.js is
  now just path constants, the write-time regression guard, and
  orchestration in main(). One small behavior-neutral cleanup along the
  way: replaced the hoisted-closure `bundlesFor_` mutable-variable hack
  with a normal parameter, since extractClassesFromFile now takes
  bundlesFor directly. No logic changed otherwise - verified by diffing
  docs/api-index.json, docs/api-index.md, docs/token-index.json, and
  token-registry.json byte-for-byte against a pre-refactor snapshot
  (empty diff on all four, both from a single `node
  scripts/gen-api-index.js` run and from the full `npm run docs`
  pipeline).
- SL-010 + SL-012: add regression guards before the three
  fs.writeFileSync sites that downstream tooling trusts
  (gen-api-index.js, gen-token-registry.js, gen-token-index.js) - a
  >20% entry-count drop (or, for the append-only token-registry.json,
  any drop at all) now throws instead of silently writing a truncated
  artifact. No new dependency; hand-rolled against each file's existing
  "read the previous version" logic.
- SL-008/009/011/013: new scripts/README.md documenting the
  intentionally-unenforced error-handling convention, the hand-rolled
  CLI flag parsing, the 3 environment variables the scripts read
  (SLASHED_ROOT, GITHUB_REF/GITHUB_REF_NAME, GITHUB_BASE_REF), and the
  execSync(shell:true) pattern in check-artifacts.js/changelog-release.js
  (config-sourced today, not attacker input, but worth flagging for
  future artifacts.json entries).

Verified: npm run docs before/after diff empty on every generated
artifact; check:macros, check:registry, check:llm-guide, audit:check,
lint:css, build, test:unit (44/44) all pass.
@qodo-code-review

Copy link
Copy Markdown

Qodo is busy working

Check back in a few minutes. Qodo's code review agents are on it.

Grey Divider

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b10acfc4-7b35-445b-afac-a5d1fb83537c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)

❌ Error creating Unit Test PR.

  • Create PR with unit tests
  • Commit unit tests in branch claude/audit-pr3-scripts-hardening

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Harden docs build scripts: split API index generator and add regression guards

✨ Enhancement 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Split the API index generator into dedicated extract/render modules for readability.
• Add write-time regression guards to prevent silently truncated generated artifacts.
• Document scripts conventions: error handling, flags, env vars, and execSync shell usage.
Diagram

graph TD
  npm_docs["Docs pipeline"] --> gen_api["gen-api-index.js"] --> extract["api-index extract"] --> render["api-index render"] --> api_out[("api-index artifacts")]
  inputs["Sources + previous outputs"] --> extract
  npm_docs --> gen_tokens["gen-token-*.js"] --> token_out[("token artifacts")]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Add CI snapshot tests (golden files) for generated artifacts
  • ➕ Catches unintended output drift deterministically during review/CI
  • ➕ Avoids adding runtime behavior that can block local generation in edge cases
  • ➖ Doesn't protect users running scripts locally outside CI
  • ➖ Requires maintaining golden files and managing intentional update workflows
2. Switch CSS parsing from regex/masking to a CSS AST (e.g., PostCSS)
  • ➕ More robust parsing; fewer regex edge cases and masking/index fragility
  • ➕ Easier long-term extension (selectors, at-rules, comments)
  • ➖ Much larger change surface and higher behavior-change risk
  • ➖ Adds dependency/complexity; would require extensive validation against current outputs
3. Make regression thresholds configurable (env var/flag)
  • ➕ Allows emergency/unusual bulk changes without code edits
  • ➕ More flexible for future intentional reductions
  • ➖ More knobs to document and reason about; can be misused to bypass safeguards
  • ➖ Slightly increases script API surface area

Recommendation: Current approach is a good risk-reduction step: the SL-006 split is a behavior-neutral relocation that improves maintainability, and the write-time guards directly address silent-truncation failure modes. Consider a follow-up adding CI snapshot tests for the generated docs artifacts to further lock in output equivalence without relying solely on runtime thresholds.

Files changed (6) +798 / -683

Bug fix (2) +38 / -5
gen-token-index.jsAdd token-index write-time regression guard against large count drops +25/-5

Add token-index write-time regression guard against large count drops

• Before writing docs/token-index.json, reads the previous file (if present) and throws when token count shrinks by >20%, treating it as a likely parsing/tier-set regression; syntax errors in the prior file degrade to a warning.

scripts/gen-token-index.js

gen-token-registry.jsAdd append-only registry shrink guard before writing token-registry.json +13/-0

Add append-only registry shrink guard before writing token-registry.json

• Captures the previous token count and throws if the registry would shrink, enforcing the append-only contract (tokens should be marked removed rather than deleted).

scripts/gen-token-registry.js

Refactor (3) +715 / -678
gen-api-index.jsRefactor into orchestrator + add API index entry-count regression guard +30/-678

Refactor into orchestrator + add API index entry-count regression guard

• Replaces the monolithic implementation with orchestration that delegates extraction/rendering to new modules. Adds a write-time guard that throws if total entries drop by more than 20% versus the previous generated index, preventing silent truncation.

scripts/gen-api-index.js

extract.jsNew module: API index extraction (tokens/classes/overlays/bundles) +602/-0

New module: API index extraction (tokens/classes/overlays/bundles)

• Introduces a dedicated extraction module containing the prior gen-api-index logic for reading sources, parsing banners/notes, computing bundle membership, extracting tokens and classes, and merging with annotations and the previous index.

scripts/lib/api-index/extract.js

render.jsNew module: API index Markdown rendering from assembled data +83/-0

New module: API index Markdown rendering from assembled data

• Moves Markdown companion generation into a focused renderer that consumes the same assembled data as the JSON output to prevent drift between views.

scripts/lib/api-index/render.js

Documentation (1) +45 / -0
README.mdDocument scripts conventions and risk notes (SL-008/009/011/013) +45/-0

Document scripts conventions and risk notes (SL-008/009/011/013)

• Adds a scripts-specific README covering error-handling patterns, ad-hoc CLI flag parsing, known environment variables read by scripts, and the security implications of execSync with shell execution.

scripts/README.md

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Note

Unit test generation is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.


Generating unit tests... This may take up to 20 minutes.

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Request timed out after 900000ms (requestId=4016eea8-7ecb-4c47-8615-b12f8eb7b006)

@jackgranatowski
jackgranatowski merged commit 6310621 into claude/pr-469-audit-rebase-ggp0e4 Jul 2, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants